Add escapeLiteral and escapeIdentifier functions #396
Conversation
|
Okay I fixed the tests. Just need a code review now. |
|
Sorry for not getting to this over the weekend. Had friends & family in town. With that out of the way... This pull request is one of the best pull requests I've ever received. You used a style in line with what's currently in the code base, you kept feature parity between native and JavaScript bindings, and most importantly you provided a huge amount of test coverage. ❤️ ❤️ ❤️ Thank you so much! 👍 I'll merge & release a new minor version today. |
|
Cannot find node-gyp? Do |
|
As far as the error handling in C++ land goes, you did well. The only thing you missed is MallocCString can return NULL if the call to malloc fails. I've put in code to handle this edge case. Everything else looks perfectamundo. |
|
Merged this via command-line |
|
Oh yeah, forgot about checking NULL. It has been awhile since I have touched C/C++. :) Thanks for merging! |
I needed to be able to escape SQL identifiers (table names, column names, etc), so I added
escapeLiteralandescapeIdentifier. There is a native and JS version. Below is an example usage.I am unsure the bindings C code is 100% correct. Particularly how errors are handled. The functions are synchronous so not sure I did the right thing for error handling. As far as I know, the native functions do not perform any I/O only string manipulation which is why I made them synchronous.
I would appreciate it if someone double checked my C to JS port of PQescapeLiteral and PQescapeIdentifier starting on line 3238.
http://doxygen.postgresql.org/fe-exec_8c_source.html
I added tests for both JS and native, but I am unable to run the native tests. It tells me that it cannot find node-gyp, any ideas?